Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reimplement decorations as an interval tree #36410

Merged
merged 65 commits into from
Oct 20, 2017
Merged

Reimplement decorations as an interval tree #36410

merged 65 commits into from
Oct 20, 2017

Conversation

alexdima
Copy link
Member

@alexdima alexdima commented Oct 17, 2017 •
edited
Loading

This PR reimplements decorations in the editor buffer as an interval tree which stores relative offsets. The goal is to make inserting, removing and editing be a lot faster when dealing with a large number of decorations. Memory usage savings are also a nice win.

This should help with increasing the limits for #496 and #1110

Further optimizations (on top of an interval tree):

  • storing two interval trees, one with decorations that show up in the overview ruler, one with decorations that do not. The reason is that the overview ruler paints all the decorations, not just the ones in the viewport.
  • the decorations overview ruler has been reimplemented to avoid allocations almost entirely.
  • fetching decorations to render in the viewport now eliminates hidden ranges (i.e. folded ranges) and avoids visiting decorations that would not be visible anyways.
  • the find widget, when having more than 1k results, will approximate overview ruler decorations and will create a separate set of decorations for the overview ruler.

checker.txt + Bracket Pair Colorizer ~ 38,177 decorations

Before After
removing+adding 194.5ms (52.5+56.3+85.7) 55.7ms (25.4+14.8+15.5)
typing \n (_pushEditOperations) 148.7ms 1.6ms
model mem usage 3,865,672 3,858,468
model mem usage with decorations 40,826,928 6,650,196

jquery.js + Search for e + no find limit ~ 24,034 decorations (22,974 find matches)

Before After
adding 230.3ms 38.9ms
removing 759.6ms 23.9ms
typing \n (_pushEditOperations) 33.1ms 0.7ms
model mem usage 2,667,304 2,630,636
model mem usage with decorations 15,269,076 4,228,524

@alexdima
Implement a relative interval tree
648e6f6
@alexdima
Implement interval searching
bc76ef8
@alexdima
Eliminate RelativeInterval
8e6968f
@alexdima
Reorganize code
a324329
@alexdima
Simplify IModelDecorationsChangedEvent
d902ab8
@alexdima
Reduce usages of IModelDecorationsChangedEvent.changedDecorations
6023aee
@alexdima
Eliminate IModelDecorationsChangedEvent.changedDecorations
a47159b
@alexdima
IntervalTree.insert gets passed in the node to be inserted
479958b
@alexdima
IntervalNode implements IModelDecoration
80859dd
@alexdima
Reduce usage of Interval
452e0ed
@alexdima
Implement interval searching to be non recursive and not use a stack
6b374f3
@alexdima
Add a flag that switches between decorations implementations
fedf6fd
@alexdima
some cleanup in intervalTree.ts
76c68ae
@alexdima
Document current decoration behaviour
c2da406
@alexdima
Fix issue in IntervalTree.resolveNode
10c5d96
@alexdima
Fix issue in changeDecoration
e7ae7d7
@alexdima
Adjust tests for USE_NEW_DECORATIONS
bfedda7
@alexdima
Always emit a model decorations changed event when the model is being…
167c2e7 
… edited
@alexdima
Implement IntervalTree.acceptReplace
1a37562
@alexdima
Update interval tree when edits occur
b179e24
@alexdima
Optimize deltaDecorations
d4408e2
@alexdima
Add fast path for vscode.TextEditor.setDecorations
3246501
@alexdima
Capture current side editing behaviour
5aff04b
@alexdima
Reduce reliance on _addMarker for cursor tracking
400afdb
@alexdima
Further reduce reliance on _addMarker for editing commands selection …
a939531 
…tracking
@alexdima
Flip the switch
7b9e4ae
@alexdima
Remove old decorations implementation
40f61d0
@alexdima
Remove TextModelWithMarkers
a0a5f18
@alexdima alexdima self-assigned this Oct 17, 2017
@alexdima
More aggressive merging of overview ruler decorations
69ab7b1
@alexdima
Tweak the way find decorations are managed
34e74ba
@alexdima
Approximate find matches in the overview ruler if there are a high nu…
c01d8c1 
…mber of results
@alexdima
Query model for decorations only for visible model lines in the viewp…
9146ff3 
…ort (in contiguos visible model lines batches)
@alexdima
Simplify ViewModelDecoration
819c005
@alexdima
Improve message
f2d47bc
@alexdima
Bump find limit to 20k given all the perf work around decorations
9073cc1
@alexdima
Avoid flickering by painting the overview ruler with colors in the sa…
93157c2 
…me order
@alexdima
Renames
b45c8b8
@alexdima
More overflow checks
3c2db90
@alexdima
Avoid needless checks
a0982eb
@alexdima
Increase slow test timeout
3212c45
@alexdima
SelectionHighlighter should not highlight the exact same matches as t…
057714e 
…he find widget
@alexdima
Merge remote-tracking branch 'origin/master' into alex/interval-tree
5f85622
@alexdima alexdima merged commit 55b2ff2 into master Oct 20, 2017
This was referenced Oct 20, 2017
Closed
Closed
Closed
Closed
@alexdima alexdima deleted the alex/interval-tree branch October 21, 2017 13:26
@alexdima alexdima mentioned this pull request Oct 23, 2017
Closed
@eamodio
Copy link
Contributor

eamodio commented Oct 23, 2017

@alexandrudima THIS IS AWESOME! I'm seeing a great improvement in decoration application & removal -- and even better it also seems stable (performance-wise) -- before adding a LOT of decorations would be slow-ish but ok, but removing them would sometimes get worse and worse over time.

Thank you!!!

@alexdima alexdima mentioned this pull request Oct 24, 2017
Merged
@dijonkitchen dijonkitchen mentioned this pull request Nov 10, 2017
Closed
@lkytal
Copy link

lkytal commented Nov 11, 2017

Brillant work.

@DanTup DanTup mentioned this pull request Nov 17, 2017
Closed
@alexdima alexdima mentioned this pull request Mar 29, 2018
Closed
This was referenced Apr 17, 2018
Closed
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@alexdima alexdima

Labels
None yet
Projects
None yet
Milestone
October 2017
Development

Successfully merging this pull request may close these issues.
3 participants
@alexdima @eamodio @lkytal